vm: make vm.Module.evaluate() conditionally synchronous #60205

joyeecheung · 2025-10-10T18:24:05Z

This patch:

Make sure that the vm.Module.evaluate() method is conditionally synchronous based on the specification. Previously, it's unconditionally asynchronous (even for synthetic modules and source text modules without top-level await).
Clarify the synchronicity of this method in the documentation
Add more tests for the synchronicity of this method.

codecov · 2025-10-10T19:20:49Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.55%. Comparing base (db0121b) to head (d308c89).
⚠️ Report is 34 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #60205      +/-   ##
==========================================
+ Coverage   88.53%   88.55%   +0.01%     
==========================================
  Files         704      704              
  Lines      208087   208151      +64     
  Branches    40010    40013       +3     
==========================================
+ Hits       184239   184322      +83     
+ Misses      15866    15863       -3     
+ Partials     7982     7966      -16

Files with missing lines	Coverage Δ
lib/internal/vm/module.js	`96.37% <100.00%> (+0.03%)`	⬆️

... and 42 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

doc/api/vm.md

test/parallel/test-vm-module-evaluate-source-text-module.js

lib/internal/vm/module.js

test/parallel/test-vm-module-evaluate-source-text-module.js

joyeecheung · 2025-10-12T17:49:30Z

Added a benchmark for source text modules - there are some performance improvements by removing the async-await:

                                                                      confidence improvement accuracy (*)    (**)   (***)
vm/source-text-module-leaf.js n=1000 type='async' stage='all'                ***      4.24 %       ±2.05%  ±2.72%  ±3.55%
vm/source-text-module-leaf.js n=1000 type='async' stage='compile'                    -0.58 %       ±1.54%  ±2.05%  ±2.68%
vm/source-text-module-leaf.js n=1000 type='async' stage='evaluate'           ***     31.67 %       ±8.54% ±11.40% ±14.91%
vm/source-text-module-leaf.js n=1000 type='async' stage='instantiate'                 6.96 %       ±8.02% ±10.68% ±13.90%
vm/source-text-module-leaf.js n=1000 type='async' stage='link'                        1.29 %       ±2.93%  ±3.92%  ±5.15%
vm/source-text-module-leaf.js n=1000 type='sync' stage='all'                   *      1.28 %       ±1.06%  ±1.42%  ±1.85%
vm/source-text-module-leaf.js n=1000 type='sync' stage='compile'                     -0.97 %       ±1.32%  ±1.76%  ±2.29%
vm/source-text-module-leaf.js n=1000 type='sync' stage='evaluate'            ***     15.50 %       ±3.84%  ±5.11%  ±6.65%
vm/source-text-module-leaf.js n=1000 type='sync' stage='instantiate'                  3.47 %       ±4.66%  ±6.21%  ±8.08%
vm/source-text-module-leaf.js n=1000 type='sync' stage='link'                        -1.68 %       ±2.83%  ±3.80%  ±5.01%

- Make sure that the vm.Module.evaluate() method is conditionally synchronous based on the specification. Previously, the returned promise is unconditionally pending (even for synthetic modules and source text modules without top-level await) instead of immediately fulfilled, making it harder to debug as it deviates from the specified behavior. - Clarify the synchronicity of this method in the documentation - Add more tests for the synchronicity of this method.

joyeecheung · 2025-10-13T10:32:00Z

Updated the evaluate implementation so that it keeps rejecting for loader errors yet still returns an immediately fulfilled promise for the actual evaluation. The perf numbers are still similar:

                                                                      confidence improvement accuracy (*)    (**)   (***)
vm/source-text-module-leaf.js n=1000 type='async' stage='all'                ***      3.81 %       ±1.19%  ±1.59%  ±2.07%
vm/source-text-module-leaf.js n=1000 type='async' stage='compile'                    -0.85 %       ±1.11%  ±1.48%  ±1.93%
vm/source-text-module-leaf.js n=1000 type='async' stage='evaluate'           ***     34.00 %       ±8.05% ±10.74% ±14.02%
vm/source-text-module-leaf.js n=1000 type='async' stage='instantiate'                -1.24 %       ±7.81% ±10.40% ±13.54%
vm/source-text-module-leaf.js n=1000 type='async' stage='link'                       -0.58 %       ±2.57%  ±3.44%  ±4.51%
vm/source-text-module-leaf.js n=1000 type='sync' stage='all'                          0.67 %       ±1.27%  ±1.70%  ±2.21%
vm/source-text-module-leaf.js n=1000 type='sync' stage='compile'                     -0.24 %       ±1.12%  ±1.49%  ±1.94%
vm/source-text-module-leaf.js n=1000 type='sync' stage='evaluate'            ***     19.77 %       ±3.54%  ±4.72%  ±6.14%
vm/source-text-module-leaf.js n=1000 type='sync' stage='instantiate'                  1.93 %       ±4.66%  ±6.20%  ±8.07%
vm/source-text-module-leaf.js n=1000 type='sync' stage='link'                        -0.09 %       ±3.83%  ±5.09%  ±6.63%

joyeecheung · 2025-10-13T10:42:48Z

Opened a separate issue about the rejection #60242

nodejs-github-bot · 2025-10-13T15:35:49Z

CI: https://ci.nodejs.org/job/node-test-pull-request/69713/

nodejs-github-bot · 2025-10-14T16:13:40Z

CI: https://ci.nodejs.org/job/node-test-pull-request/69725/

legendecas · 2025-10-14T20:20:08Z

doc/api/vm.md

-case it will either do nothing if the initial evaluation ended in success
-(`module.status` is `'evaluated'`) or it will re-throw the exception that the
-initial evaluation resulted in (`module.status` is `'errored'`).
+If the module is a `vm.SourceTextModule`,`evaluate()` must be called after the module has been at least linked;


The status check requires the module to be instantiated.

node/lib/internal/vm/module.js

Lines 224 to 226 in 170848b

if (status !== kInstantiated &&

status !== kEvaluated &&

status !== kErrored) {

Suggested change

If the module is a `vm.SourceTextModule`,`evaluate()` must be called after the module has been at least linked;

If the module is a `vm.SourceTextModule`, `evaluate()` must be called after the module has been instantiated;

legendecas · 2025-10-14T20:23:05Z

doc/api/vm.md

+   fulfilled _synchronously_ after the module and all its dependencies have been evaluated.
+   1. If the evaluation succeeds, the promise will be _synchronously_ resolved to `undefined`.
+   2. If the evaluation results in an exception, the promise will be _synchronously_ rejected with the exception
+      that causes the evaluation to fail.


Worth directing users to infer module.error, instead of inspecting the promise.

Suggested change

that causes the evaluation to fail.

`module.error` that causes the evaluation to fail and .

nodejs-github-bot added needs-ci PRs that need a full CI run. vm Issues and PRs related to the vm subsystem. labels Oct 10, 2025

legendecas reviewed Oct 12, 2025

View reviewed changes

doc/api/vm.md Outdated Show resolved Hide resolved

test/parallel/test-vm-module-evaluate-source-text-module.js Show resolved Hide resolved

lib/internal/vm/module.js Show resolved Hide resolved

legendecas reviewed Oct 12, 2025

View reviewed changes

test/parallel/test-vm-module-evaluate-source-text-module.js Show resolved Hide resolved

benchmark: add benchmark for leaf source text modules

5237650

joyeecheung force-pushed the vm-evaluate branch from 63566f6 to a482e51 Compare October 12, 2025 17:51

joyeecheung force-pushed the vm-evaluate branch from a482e51 to d308c89 Compare October 13, 2025 10:30

joyeecheung mentioned this pull request Oct 13, 2025

vm.Module.evaluate() morphs loader errors into module evaluation rejections #60242

Open

legendecas approved these changes Oct 14, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

vm: make vm.Module.evaluate() conditionally synchronous #60205

vm: make vm.Module.evaluate() conditionally synchronous #60205

joyeecheung commented Oct 10, 2025

Uh oh!

codecov bot commented Oct 10, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joyeecheung commented Oct 12, 2025

Uh oh!

joyeecheung commented Oct 13, 2025

Uh oh!

joyeecheung commented Oct 13, 2025

Uh oh!

nodejs-github-bot commented Oct 13, 2025

Uh oh!

nodejs-github-bot commented Oct 14, 2025

Uh oh!

legendecas Oct 14, 2025

Uh oh!

legendecas Oct 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	if (status !== kInstantiated &&
	status !== kEvaluated &&
	status !== kErrored) {

	If the module is a `vm.SourceTextModule`,`evaluate()` must be called after the module has been at least linked;
	If the module is a `vm.SourceTextModule`, `evaluate()` must be called after the module has been instantiated;

	that causes the evaluation to fail.
	`module.error` that causes the evaluation to fail and .

Uh oh!

vm: make vm.Module.evaluate() conditionally synchronous #60205

Are you sure you want to change the base?

vm: make vm.Module.evaluate() conditionally synchronous #60205

Conversation

joyeecheung commented Oct 10, 2025

Uh oh!

codecov bot commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joyeecheung commented Oct 12, 2025

Uh oh!

joyeecheung commented Oct 13, 2025

Uh oh!

joyeecheung commented Oct 13, 2025

Uh oh!

nodejs-github-bot commented Oct 13, 2025

Uh oh!

nodejs-github-bot commented Oct 14, 2025

Uh oh!

legendecas Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

legendecas Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Oct 10, 2025 •

edited

Loading